Skip to content

fix(stream): emit a single finish_reason for tool-call chat completions#114

Open
zerone-jang wants to merge 1 commit into
RayBytes:mainfrom
zerone-jang:fix/single-finish-reason-for-tool-calls
Open

fix(stream): emit a single finish_reason for tool-call chat completions#114
zerone-jang wants to merge 1 commit into
RayBytes:mainfrom
zerone-jang:fix/single-finish-reason-for-tool-calls

Conversation

@zerone-jang

Copy link
Copy Markdown

Summary

When a streamed /v1/chat/completions response contains a tool call, sse_translate_chat currently emits two finish_reason chunks for the same choice:

  1. finish_reason: "tool_calls" — emitted right after each completed function_call (and web_search_call) item, and
  2. a trailing finish_reason: "stop" — emitted unconditionally from the response.completed handler, because sent_stop_chunk is only ever set on the (unreachable, see note below) response.output_text.done branch and inside response.completed itself.

Per the OpenAI streaming format, a choice carries exactly one terminal finish_reason. This PR marks the stop chunk as already sent when the tool_calls finish chunk goes out, so response.completed no longer appends the spurious "stop".

Why it matters (real-world impact)

Clients that honor the last finish_reason classify the response as a plain-text turn. Some agent frameworks then silently discard the accumulated tool calls whenever the model prefixed them with visible commentary text (which GPT-5.x models do routinely — "Let me look that up." + tool call). The observable symptom is an assistant that keeps announcing work ("I'll fetch those articles now.") and never executes it, since its tool calls evaporate at the proxy boundary.

We debugged exactly this in production with OpenClaw as the client: the model verifiably emitted function_call items upstream (visible with --verbose-obfuscation), but every response that mixed commentary text with tool calls was delivered to the agent as text-only. Responses consisting of a bare tool call survived — which made the failure look maddeningly nondeterministic.

Repro

Any streaming request where the model produces preamble text before the call:

curl -N http://127.0.0.1:8000/v1/chat/completions -H "Content-Type: application/json" -d '{
  "model": "gpt-5.4", "stream": true,
  "messages": [{"role":"user","content":"Weather in Seoul? Say one sentence about what you will do, then use the tool."}],
  "tools": [{"type":"function","function":{"name":"get_weather","parameters":{"type":"object","properties":{"city":{"type":"string"}},"required":["city"]}}}]
}'

Observed tail of the stream before this fix:

data: {... "delta": {"tool_calls": [...]}, "finish_reason": null}
data: {... "delta": {}, "finish_reason": "tool_calls"}
data: {... "delta": {}, "finish_reason": "stop"}      <-- spurious
data: [DONE]

After the fix the "stop" chunk is gone; plain-text responses are unaffected (their stop chunk still comes from response.completed).

Changes

  • chatmock/utils.py: set sent_stop_chunk = True after emitting the finish_reason: "tool_calls" chunk, in both the web_search_call path and the response.output_item.done function_call path (2 lines).
  • tests/test_routes.py: streaming regression test asserting that a mixed text + function_call response yields the tool-call delta and exactly ["tool_calls"] as finish reasons. Fails without the fix, passes with it.

Note for reviewers

There is an elif kind == "response.output_text.done": branch that emits an early finish_reason: "stop" — it is currently unreachable because the preceding elif isinstance(kind, str) and kind.endswith(".done"): pass catch-all shadows it. Please don't revive that branch as part of cleaning this up: emitting stop at text-done would guarantee the tool-call-after-commentary case breaks (text completes before the function_call item arrives).

How to try locally

python -m unittest tests.test_routes tests.test_models   # 23 tests, all pass
python -m unittest tests.test_routes.RouteTests.test_chat_completions_stream_single_finish_reason_with_tool_call

Manual verification: run chatmock serve, issue the curl above, and check the stream tail contains a single finish_reason.

Docs (README.md / DOCKER.md): not required — no user-facing flags or payload shapes change; this restores spec-conformant streaming.

sse_translate_chat emitted finish_reason "tool_calls" for each completed
function_call item and then an additional finish_reason "stop" chunk from
the response.completed handler. Per the OpenAI streaming format a choice
carries exactly one finish_reason; clients that honor the last one classify
a mixed text + tool_call response as a plain text turn and silently drop
the accumulated tool calls.

Mark the stop chunk as already sent when the tool_calls finish chunk goes
out, so response.completed no longer appends the spurious "stop".

Adds a streaming regression test that fails without the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant